Skip to content

fix(dag): enforce execution-location ownership on adoption, recovery, and wake (DAG-LOC-01) - #266

Merged
LeXwDeX merged 8 commits into
devfrom
fix/dag-loc-ownership
Aug 14, 2026
Merged

fix(dag): enforce execution-location ownership on adoption, recovery, and wake (DAG-LOC-01)#266
LeXwDeX merged 8 commits into
devfrom
fix/dag-loc-ownership

Conversation

@LeXwDeX

@LeXwDeX LeXwDeX commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Closes #238.

What

Sibling worktrees share a project id, so projectId-only guards let the wrong directory adopt/recover/wake/spawn each other's workflows (six RED scenarios in the issue). This installs a single execution-location authority:

  • packages/opencode/src/dag/location.tsownsWorkflow/ownsSession re-read the durable WorkflowTable.directory stamp (realpath-normalized) on every check; project id is only the fast-reject. Session-sourced stamping at dag.create (session's durable directory, not the ambient request instance — dag.ts:410-413) so a request on directory A can never stamp a workflow for B's session.
  • WorkflowTable.directory column + migration 20260813040429_workflow_directory with session-join backfill; NULL stamps match no instance (fail-closed).
  • All adoption/recovery/wake guards in the runtime loop route through the authority; goal-side ownership (Goal.ownsSession) closes the goal-only-session vacuous-own hole; deadline-watcher revalidation survives transient store defects (R13 pattern).

Evidence

  • 7 deterministic probes in test/dag/dag-location-guards.test.ts and friends: C1 concurrent adoption, C2 remove-vs-wake, C3 cascade-window orphan (resolved SAFE with argument), C4 SessionMoved wedge pin, C5 post-deletion replay, C6 recoverOrphanPending-vs-remove, R7-ext write-once static barrier. 5 consecutive flake runs 20/20.
  • Gates: typecheck (core+opencode), bun test test/dag test/goal test/tool 930/0, root lint 4850/4852 (ratchet lowered, not bumped).

Known limitations (pinned, not fixed — by design out of scope)

  • SessionMoved wake wedge (C4): workflows with mixed directory stamps across a session move have NO directory owner — wake is deliberately wedged rather than mis-delivered. Needs an atomic-DB-adoption redesign for a real fix; spawnReady eviction is the mitigation. Human follow-up welcome.
  • vs-deletion tail: adoption racing a workflow's own deletion can still slip within the last revalidation window; spawnReady eviction mitigates. Same atomic-adoption redesign territory.
  • Legacy packages/app/src/utils/id.ts encoder (desktop-side, pre-existing) — follow-up candidate.

Review status

Independent standards+intent review arbitration returned REJECT with a single decisive gap: the H1 duplicate-publish latch commit (959bae7) lacks a mutation-falsifiable probe. That probe is being added on this branch before merge; CI on this PR gates the merge.

LeXwDeX and others added 8 commits August 15, 2026 00:48
… and wake (DAG-LOC-01)

The DAG runtime is per-directory InstanceState, but the durable store, the
event bus, and the workflow rows are process-global. Guards keyed on the
PROJECT ID let sibling worktrees of one project (same id, distinct
directories) all adopt, recover-cancel, wake, and spawn for each other's
workflows. This change installs a single execution-location authority and
routes every adoption/recovery/wake guard through it.

Authority — packages/opencode/src/dag/location.ts (single module):
- ownsWorkflow(workflowID, directory): re-reads the durable workflow row on
  every check; project id is the fast-reject, the stamped DIRECTORY
  (realpath-normalized, raw-path fallback) is the deciding guard. R6 identity
  revalidation falls out of the re-read: a repainted project_id stops the
  stale in-memory entry from publishing transitions.
- ownsSession(sessionID, directory): every durable workflow row of the
  session must match (vacuous-true for workflow-less sessions so goal-only
  sessions keep working). Key lives on the workflow row only — no
  session-table reads (R7's negative half).
- Database is resolved lazily via Effect.serviceOption so the loops' static
  layer requirements stay unchanged (the optional-cross-dependency pattern).

Join vs column: the round-1 analysis allowed either. R7 mandates the key on
the workflow row itself and forbids session.directory reads in dag sources,
so the column wins: WorkflowTable.directory, stamped at dag.create from the
creating instance (WorkflowCreated.directory, optional for legacy decodes),
plus migration 20260813040429_workflow_directory (generated by
script/migration.ts) with a session-join backfill so in-flight workflows
survive upgrades. A NULL stamp matches no instance (never adopted).

Guard regions replaced in packages/opencode/src/dag/runtime/loop.ts:
- recoverWorkflow (~L328): projectId guard -> ownsWorkflow(wf.id, ctx.directory)
- recoverOrphanPending (~L439): same replacement
- WorkflowStarted first-wave adoption (~L487): same replacement
- startup wake sweep (~L1308): snapshot projectId check -> ownsSession
- tryDeliverWake entry (~L1048, previously unguarded): ownsSession
- checkCompletion (~L282): new revalidation gate (R6)
- SessionV1.Event.Deleted teardown subscription (~L1233, R5): drops the
  session's runtime entries and interrupts their fibers/watchers, mirroring
  the workflow-terminal cleanup pattern
GoalLoop idle trigger (packages/opencode/src/goal/loop.ts ~L120): routed
through ownsSession — aligns the idle path with the directory-scoped goal
scan.

Probes (test/dag/dag-location-guards.test.ts): RED 7/7 before
(/tmp/dag-loc-red-full.log), GREEN 7/7 after — R1 adoption, R2 startup
recovery, R3 idle wake, R4 startup sweep, R5 deletion teardown, R6 identity
migration, R7 static contract. R5/R6's negative-window assertions used
pollWithTimeout (a positive-wait tool whose timeout errors the effect, so the
"nothing must happen" outcome could never pass); their mechanics were fixed
to settle-then-sleep-and-assert with identical intent. Pre-existing seeds
gained the directory stamp (dag-wake-integration, dag-adoption-step-races,
dag-orphan-pending-recovery, workflow-tool/summary-publisher fixtures).

Mutations: bypassing the tryDeliverWake authority guard -> R3 red (6 pass);
removing the Deleted teardown subscription -> R5 red (6 pass); both restored.

Verification: packages/opencode test/dag + test/goal = 560 pass / 0 fail;
core dag-projector-drift + dag-store-summaries pass; test:dag-core pass;
test:httpapi 227 pass / 0 fail; bun typecheck clean (root); bun lint 4850
(ratchet tightened 4852 -> 4850: probe harness's `as never` fixture shims
file-scoped suppressed like the dag-loop-guards template; two pre-existing
`as never` casts replaced); check:generated clean for sdk/js and client.

Co-Authored-By: Claude <noreply@anthropic.com>
…alidation, session-sourced stamp (DAG-LOC-01 follow-up)

Two-lens review follow-up on 3498dd670. All six introduced P2s closed; each is
pinned by a probe (or an argument where the defect is structurally
unobservable).

P2-A (goal vacuous-true): the dag-side ownsSession is keyed on workflow rows
and is vacuously true for goal-only sessions, so instance B could drive
instance A's goal-only sessions (cross-directory continuation, double judge,
spurious pauses). Fix: Goal.ownsSession (goal.ts) — a REAL directory check
against the durable session row (SessionTable.directory; legal there — the
goal module is outside the dag trees R7 scans). Vacuous-own remains only
where no durable answer exists (rowless synthetic sessions, or a runtime
graph without Database). The session-row read lives in a new session-domain
accessor (packages/opencode/src/session/location.ts, sessionDirectory) so
the R7 constraint (no session.directory reads in dag sources) holds and
Dag.create (P2-F) shares the same single source. Applied in the GoalLoop
idle handler (goal/loop.ts).

P2-B (goal idle subscription killable): the new guard was the first
defect-capable durable read in the goal idle handler; a store defect would
have permanently killed the runForEach subscription (Effect.ignore does not
absorb defects). Fix: the handler body is wrapped in catchCause with a
logged warning — a store defect degrades to a skipped evaluation, never a
dead loop (mirrors DagLoop's guarded()).

P2-C (R6 gates only checkCompletion): after an identity repaint the stale
entry could still win nodeQueued and materialize a child under the stale
directory, and the deadline watcher could still write escalations.
Fix: spawnReady revalidates ownership at its entry (all seven spawn call
sites funnel through it) and drops the stale entry when ownership no longer
holds; makeDeadlineWatcher revalidates before its write section (escalate +
cap enforcement) and ends its mandate on ownership loss — the check only
runs when it can disprove ownership (instance context and Database present),
so supervision still never ends in graphs without them (R13).

P2-D (NULL zombie — silent): workflows created by old builds after the
one-shot backfill keep directory=NULL and are skipped silently forever.
Fix: DagLocation logs a WARN, deduped per workflow per process, whenever an
adoption/recovery/wake path skips a NULL-directory row. The conservative
never-match policy is unchanged.

P2-E (Deleted sweep race): recoverWorkflow could pass the ownership guard,
the session deletion could cascade the rows and run the Deleted sweep before
the entry was published, then runtimes.set leaked an inert entry forever.
Fix: ownership is re-checked after the recovery body and before
runtimes.set (same for the WorkflowStarted first-wave path, whose getNodes
yield opens the same window); the ensuring still clears the recovering
reservation. No probe: the leak is behaviorally inert (every post-deletion
stimulus is filtered by runtimes.has or no-ops against the missing row), so
the race is not observably constructible — the re-check closes it
structurally.

P2-F (create boundary): Dag.create stamped the ambient REQUEST instance's
directory, so a request on directory A could create a workflow for B's
session stamped A, orphaning it from B's loops. Fix: the stamp now comes
from the TARGET session's durable directory (sessionDirectory), falling
back to the ambient instance only when the session has no durable row.
The API validation tightening was left out of this slice (HTTP handler
territory); the stamp fix is the required part.

Probes (test/dag/dag-location-guards.test.ts, "DAG-LOC-01 P2 follow-ups"):
12/12 green (7 original + P2-A, P2-F, P2-C, P2-B, P2-D). P2-B injects a
one-shot synchronous store defect through a Database proxy and proves the
NEXT idle event is still evaluated; P2-D captures the warning via
Effect.withLogger and asserts exactly one emission for two checks.

Mutations (all restored):
- bypass the goal-side guard -> P2-A red (P2-B also red: its defect lands
  on the guard's read), 10 pass
- revert the create stamp to the ambient instance -> P2-F red, 11 pass
- bypass the spawnReady revalidation -> P2-C red, 11 pass

Verification: packages/opencode test/dag + test/goal = 565 pass / 0 fail;
test:dag-core pass; test:httpapi 227 pass / 0 fail; bun typecheck clean
(root); bun lint 4850 / 0 errors (ratchet unchanged).

Co-Authored-By: Claude <noreply@anthropic.com>
…(DAG-LOC-01 follow-up)

The deadline watcher's ownership revalidation (makeDeadlineWatcher, the DAG-LOC-01 P2-C write-section gate) was the only store read in the watcher without R13 protection: ownsWorkflow's orDie read defects on a transient store failure, the outer catchCause logs it and completes the fiber, and deadline supervision ends permanently for a still-running node — no escalation, no cap, unbounded run; nothing re-forks the watcher.

Fix: wrap the revalidation in the same exit+retry pattern as the watcher's readNode (1 attempt + 3 retries with 500ms backoff, then log-and-continue). A failed read is now "cannot disprove ownership" — supervision continues — and only a POSITIVE ownership loss (successful read returning false) ends the mandate. The instance/Database presence gate is unchanged: absent either, the check does not run and supervision must not end (R13).

Probe (red-first, deterministic): new P2-watcher probe in test/dag/dag-location-guards.test.ts drives makeDeadlineWatcher through the same direct-call seam the R13 watcher tests use — readNode is a mock with no Database traffic, so the watcher's only real store query is the ownership-revalidation read, and a one-shot synchronous select defect (Proxy Database, disarmed after one hit) lands exactly there. The node is past its deadline; the probe asserts the watcher still escalates (supervision survived). Red on HEAD ("watcher ended deadline supervision after a transient ownership-revalidation store defect"), green with the fix; stashing the fix re-trips the probe red, restored it is green alongside all 13 dag-location-guards probes.

Verification: bun test test/dag test/goal 566 pass / 0 fail (incl. the existing R13 watcher tests), bun typecheck clean, bun lint 4850 warnings / 0 errors (ratchet unchanged).

Co-Authored-By: Claude <noreply@anthropic.com>
… invariant (DAG-LOC-01 rebase integration)

Root cause: the GOAL-FP-01 lease-lifecycle tests seed workflow rows via
raw SQL with no execution-location stamp. The rebased DAG-LOC-01
ownership authority is fail-closed on NULL-directory rows (P2-D zombie
policy: never adopted, recovered, or woken), so the startup wake sweep
stopped registering the ghost row and the runtime-less terminal-release
test lost its swept-registration precondition.

Fix: stamp all four seed rows with the instance directory
(process.cwd(), matching InstanceRef in the harness) — the same idiom
the sibling guard tests use — so the rows represent legitimately owned
workflows and the lease assertions exercise their original intent.
…ublish races (DAG-LOC-01 H1)

Root cause: the WorkflowStarted handler was the only adoption path
without an in-flight reservation — recoverWorkflow and
recoverOrphanPending both reserve the `recovering` slot before their
first yield, but the live handler checked the runtimes/recovering guard
and then yielded through getWorkflow/getNodes with nothing reserved.
Two concurrent WorkflowStarted events (a duplicate publish racing the
live handler) both passed the guard and both reached runtimes.set; the
second overwrote the first entry, orphaning its fibers/watchers from
every interrupt sweep and double-registering the automation lease.

Fix: reserve recovering.add(dagID) synchronously right after the guard
(no yield between check and add, so the loser's guard observes the
reservation) and release it via Effect.ensuring — exactly mirroring
recoverWorkflow's idempotency discipline. The latch also supersedes the
WorkflowReplanned no-entry re-adoption race: a replan arriving
mid-adoption drops out of recoverWorkflow instead of overwriting the
entry, and the adoption's own getNodes reads the already-replanned rows.

The vs-deletion tail of the P2-E window is NOT closed here (that would
require DB-level atomic adoption = ownership-token redesign, out of
scope pre-clustering); eviction-on-next-stimulus in spawnReady's
ownership revalidation is the accepted mitigation for that remainder.
…once barrier (DAG-LOC-01)

Pin the four evidence questions the hardening review left open, each on a
deterministic seam (no timing dependence):

- C1 concurrent live adoption: both instances booted before the workflow
  exists; only the stamped directory adopts and spawns, the sibling does not.
- C3 cascade-in-window orphan: direct row deletion (no Deleted event) leaves
  the live entry unreachable by the sweep; later stimuli spawn nothing — every
  action path revalidates ownership against the missing row.
- C4 moved-session wedge pin: mixed directory stamps across a session's
  workflows leave NO directory owner (create-time-stamp semantics; re-stamping
  on SessionEvent.Moved stays out of scope pre-clustering).
- C5 teardown replay idempotency: replaying a deleted workflow's durable
  journal through EventV2 replay does not resurrect the read-model (seq dedup
  skips projection).
- R7-ext static barrier: the directory stamp is write-once (no UPDATE writes
  it anywhere in the dag trees) and spawnReady / checkCompletion /
  makeDeadlineWatcher / the GoalLoop idle guard each carry the ownership
  authority.
…k gates (DAG-LOC-01)

The two remaining evidence questions were async negative-test barriers —
failure absorption that had no regression probe because the race window
was not deterministically reachable. Add two reusable park gates to the
two-instance harness and pin both paths through them:

- parkGetNodes: every DagStore.getNodes call flags parked and awaits a
  caller promise before delegating, so a probe can interleave a mutation
  inside a recovery sequence.
- parkWakeDelivery: SessionPrompt.prepareIfIdle (the wake-delivery
  admission seam tryDeliverWake actually uses) parks the admission result
  on a caller promise before release, and afterwards returns none while
  still counting the call.

Probes:
- C2 Session.remove racing an in-flight wake: delete the session while the
  wake delivery is parked; the raced defect is absorbed by tryDeliverWake's
  catchCause (no escape, wakeInFlight freed) and the idle wake subscription
  still processes a later session's idle (prepareIfIdle called again).
- C6 recoverOrphanPending racing Session.remove: delete the session while
  the orphan sweep is parked between getNodes and dag.fail; dag.fail on the
  gone workflow fails, the startup-scan catchCause absorbs it, the
  Effect.ensuring frees the recovering slot, and init completes cleanly.
…tch (DAG-LOC-01)

Root cause of the coverage gap: the recovering-reservation latch added in
959bae7 had no mutation-falsifiable coverage — reverting it left all 930
tests green, so the REJECT review could not prove the latch matters.

The probe parks the owner's live WorkflowStarted adoption at the
getWorkflow/getNodes seam (parkGetNodes harness, now with a call counter)
and publishes a reentrant WorkflowReplanned from the sibling directory's
ambient context. Within one subscription duplicate WorkflowStarted events
are serialized by Stream.runForEach, so the falsifiable duplicate-publish
race is the replan's no-entry recoverWorkflow path on a separate
subscription fiber — exactly the second adoption the latch repels.
Assertions: exactly one adoption sequence at the seam while parked (single
runtimes.set / single lease registration-to-be), one first-wave spawn, and
a follow-up duplicate WorkflowStarted from the sibling directory driving
no second adoption, re-spawn, or cancel.

RED proof (scratch worktree, only 959bae7 reverted): the probe fails at
'expect(adoptionsAtTheSeam).toBe(1)' with 'Expected: 1, Received: 2' —
the replan's recoverWorkflow parked a second gated getNodes inside
reconcileWorkflow and double-adopted (spawn transition rejected, loser
child cancelled).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant